Skip to content

Support size suffixes in config #893

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
cthulhu-rider merged 2 commits into from
Oct 8, 2021
Merged

Support size suffixes in config #893

cthulhu-rider merged 2 commits into from
Oct 8, 2021

Conversation

@fyrchik
Copy link
Contributor

@fyrchik fyrchik commented Oct 7, 2021 •
edited
Loading

Fix #857 .

@codecov
Copy link

codecov bot commented Oct 7, 2021 •
edited
Loading

Codecov Report

Merging #893 (19dc15b) into master (6b1e645) will increase coverage by 0.15%.
The diff coverage is 94.11%.

❗ Current head 19dc15b differs from pull request most recent head 2a2bb5d. Consider uploading reports for the commit 2a2bb5d to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #893      +/-   ##
==========================================
+ Coverage   39.16%   39.32%   +0.15%     
==========================================
  Files         244      244              
  Lines       12178    12212      +34     
==========================================
+ Hits         4770     4802      +32     
- Misses       7013     7014       +1     
- Partials      395      396       +1
Impacted Files Coverage Δ
cmd/neofs-node/config/cast.go 89.87% <94.11%> (+3.20%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6b1e645...2a2bb5d. Read the comment docs.

@fyrchik
Copy link
Contributor Author

fyrchik commented Oct 7, 2021

See also spf13/cast#129 .

@532910
Copy link

532910 commented Oct 7, 2021

I believe at least terabytes should be supported. BTW, what about floats, like .5Tb for example?

cthulhu-rider
cthulhu-rider reviewed Oct 8, 2021
View reviewed changes
cthulhu-rider
cthulhu-rider approved these changes Oct 8, 2021
View reviewed changes
Copy link
Contributor

@cthulhu-rider cthulhu-rider left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fyrchik pls don't forget to fix linter's remark

@fyrchik
Copy link
Contributor Author

fyrchik commented Oct 8, 2021

@532910 floats are not supported, at least for now. I will add TB suffixes.

fyrchik added 2 commits October 8, 2021 10:57
@fyrchik
[nspcc-dev#857] config: allow to parse sizes with suffix
796d78d 
The code is taken from viper as `cast` package that we use
doesn't have needed converter.

Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
@fyrchik
[nspcc-dev#857] config: use size suffixes where possible
2a2bb5d 
Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
@cthulhu-rider cthulhu-rider merged commit 00f14d4 into nspcc-dev:master Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cthulhu-rider cthulhu-rider cthulhu-rider approved these changes

@alexvanin alexvanin Awaiting requested review from alexvanin

@532910 532910 Awaiting requested review from 532910

@carpawell carpawell Awaiting requested review from carpawell carpawell is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

size multiplicative suffixes support

3 participants

@fyrchik @532910 @cthulhu-rider